feat: efm v2 support #930

JuanLeee · 2025-07-31T18:28:02Z

Description

Adding support for enhanced host monitoring V2 with documentation and tests.
Tested manual E2E tests and integration tests.

Porting from JDBC efm v2 implementation.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

# Conflicts: # aws_advanced_python_wrapper/plugin_service.py # aws_advanced_python_wrapper/utils/atomic.py

karenc-bq · 2025-08-13T00:22:09Z

aws_advanced_python_wrapper/utils/atomic.py

+            self._value = value
+
+
+class AtomicReference:


does it make sense to use generics here instead of Any? might be an overkill

docs/using-the-python-driver/using-plugins/UsingTheHostMonitoringPlugin.md

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

karenc-bq · 2025-08-14T00:05:15Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+            logger.warning("HostMonitorV2.MonitorIsStopped", self._host_info.host)
+
+        current_time_ns = perf_counter_ns()
+        start_monitoring_time_ns = ((current_time_ns + self._failure_detection_time_ns) // 10**9) * 10**9


Suggested change

start_monitoring_time_ns = ((current_time_ns + self._failure_detection_time_ns) // 10**9) * 10**9

start_monitoring_time_ns = current_time_ns + self._failure_detection_time_ns

Can we create helper methods for these time conversions to make the intent more clear

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

aaron-congo · 2025-08-14T23:31:31Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+
+    def __init__(self, connection: Connection):
+        self._connection_to_abort: AtomicReference = AtomicReference(weakref.ref(connection))
+        self._node_unhealthy: AtomicBoolean = AtomicBoolean(False)


in jdbc we mix the terms "node" and "host", but we should probably pick one and stick with it, in Python I think we decided on "host", can you change the references to "node" in this PR to "host"?

Just as an fyi there is some use of "node" in the LimitlessRouterMonitor messages as well - not sure if those should also be updated eventually

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

aaron-congo · 2025-08-15T00:24:12Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+                self._is_unhealthy = True
+                return
+
+            logger.debug("HostMonitorV2.HostNotResponding", self._host_info.host, self._failure_count)


Suggested change

logger.debug("HostMonitorV2.HostNotResponding", self._host_info.host, self._failure_count)

logger.debug("HostMonitorV2.HostNotResponding", self._host_info.host, self._failure_count)

return

aaron-congo · 2025-08-15T00:41:35Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+
+    def abort_connection(self, connection: Connection) -> None:
+        try:
+            connection.close()


Does this need to be wrapped in a timeout? I'm not sure if it would freeze indefinitely if the network was down

Should we call dialect.abort_connection here instead?

aaron-congo · 2025-08-15T00:41:59Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+        if context.should_abort():
+            context.set_inactive()
+            try:
+                connection_to_abort.close()


Same questions as above

aaron-congo · 2025-08-15T00:42:22Z

aws_advanced_python_wrapper/resources/aws_advanced_python_wrapper_messages.properties

+HostMonitorV2.StartMonitoringThread=[HostMonitorV2] Start monitoring thread for '{}'.
+HostMonitorV2.OpeningMonitoringConnection=[HostMonitorV2] Opening a monitoring connection to '{}'
+HostMonitorV2.OpenedMonitoringConnection=[HostMonitorV2] Opened monitoring connection: '{}'
+HostMonitorV2.ExceptionAbortingConnection=[HostMonitorV2] Exception during aborting connection: '{}'


Suggested change

HostMonitorV2.ExceptionAbortingConnection=[HostMonitorV2] Exception during aborting connection: '{}'

HostMonitorV2.ExceptionAbortingConnection=[HostMonitorV2] Exception while aborting connection: '{}'

aaron-congo · 2025-08-15T00:44:55Z

docs/using-the-python-driver/using-plugins/UsingTheHostMonitoringPlugin.md

+# Host Monitoring Plugin v2
+
+Host Monitoring Plugin v2, also known as `host_monitoring_v2`, is an alternative implementation of enhanced failure monitoring and it is functionally equal to the Host Monitoring Plugin described above. Both plugins share the same set of [configuration parameters](#enhanced-failure-monitoring-parameters). The `host_monitoring_v2` plugin is designed to be a drop-in replacement for the `host_monitoring` plugin.
+The `host_monitoring_v2` plugin can be used in any scenario where the `host_monitoring` plugin is mentioned. This plugin is enabled by default. The original EFM plugin can still be used by specifying `host_monitoring` in the `plugins` parameter.


I think we need to update the DEFAULT_PLUGINS value in utils\properties.py so that it uses the new plugin by default instead of v1

aaron-congo · 2025-08-15T00:48:35Z

I recently added an EFM2 integration test to JDBC, could you please port it over as well?
https://github.com/aws/aws-advanced-jdbc-wrapper/blob/main/wrapper/src/test/java/integration/container/tests/EFM2Test.java

sophia-bq · 2025-08-14T22:56:29Z

aws_advanced_python_wrapper/resources/aws_advanced_python_wrapper_messages.properties

+HostMonitoringV2Plugin.HostInfoNone=[HostMonitoringV2Plugin] Could not find HostInfo to monitor for the current connection.
+HostMonitoringV2Plugin.ConfigurationNotSupported=[HostMonitoringV2Plugin] Aborting connections from a separate thread is not supported for the detected driver dialect: '{}'. The EFM V2 plugin requires this feature to be supported.
+HostMonitoringV2Plugin.UnableToIdentifyConnection=[HostMonitoringV2Plugin] Unable to identify the connected database instance: '{}', please ensure the correct host list provider is specified. The host list provider in use is: '{}'.
+


I think there are a couple unnecessary new lines in this file

sophia-bq · 2025-08-14T23:07:44Z

aws_advanced_python_wrapper/resources/aws_advanced_python_wrapper_messages.properties

+HostMonitorV2.HostNotResponding=[HostMonitorV2] Host '{}' is not *responding* '{}'.
+HostMonitorV2.HostAlive=[HostMonitorV2] Host '{}' is *alive*.
+
+MonitorServiceV2.ExceptionAbortingConnection=[MonitorServiceV2] Exception during aborting connection: '{}'


Suggested change

MonitorServiceV2.ExceptionAbortingConnection=[MonitorServiceV2] Exception during aborting connection: '{}'

MonitorServiceV2.ExceptionAbortingConnection=[MonitorServiceV2] Exception while aborting connection: '{}'

sophia-bq · 2025-08-14T23:08:17Z

aws_advanced_python_wrapper/resources/aws_advanced_python_wrapper_messages.properties

+HostMonitorV2.HostAlive=[HostMonitorV2] Host '{}' is *alive*.
+
+MonitorServiceV2.ExceptionAbortingConnection=[MonitorServiceV2] Exception during aborting connection: '{}'
+MonitorServiceV2.HostNotResponding=[MonitorServiceV2] Host '{}' is not *responding* '{}'.


This is a duplicate of 197

sophia-bq · 2025-08-14T23:10:18Z

docs/using-the-python-driver/using-plugins/UsingTheHostMonitoringPlugin.md

+The `host_monitoring_v2` plugin can be used in any scenario where the `host_monitoring` plugin is mentioned. This plugin is enabled by default. The original EFM plugin can still be used by specifying `host_monitoring` in the `plugins` parameter.
+
+> [!NOTE]\
+> Since these two plugins are separate plugins, users may decide to use them together with a single connection. While this should not have any negative side effects, it is not recommended. It is recommended to use either the `host_monitoring` plugin, or the `host_monitoring_v2` plugin where it's needed.


Nit:

Suggested change

> Since these two plugins are separate plugins, users may decide to use them together with a single connection. While this should not have any negative side effects, it is not recommended. It is recommended to use either the `host_monitoring` plugin, or the `host_monitoring_v2` plugin where it's needed.

> Since these two plugins are separate plugins, users may decide to use them together with a single connection. While this should not have any negative side effects, it is not recommended. It is recommended to use either the `host_monitoring_v2` plugin, or the `host_monitoring` plugin where it's needed.

sophia-bq · 2025-08-14T23:16:19Z

docs/using-the-python-driver/using-plugins/UsingTheHostMonitoringPlugin.md

+# Host Monitoring Plugin v2
+
+Host Monitoring Plugin v2, also known as `host_monitoring_v2`, is an alternative implementation of enhanced failure monitoring and it is functionally equal to the Host Monitoring Plugin described above. Both plugins share the same set of [configuration parameters](#enhanced-failure-monitoring-parameters). The `host_monitoring_v2` plugin is designed to be a drop-in replacement for the `host_monitoring` plugin.
+The `host_monitoring_v2` plugin can be used in any scenario where the `host_monitoring` plugin is mentioned. This plugin is enabled by default. The original EFM plugin can still be used by specifying `host_monitoring` in the `plugins` parameter.


The value DEFAULT_PLUGINS = "aurora_connection_tracker,failover,host_monitoring"
in properties.py needs to be updated for this to be true. Should also double check other documentation for mentions of default plugins and update those

sophia-bq · 2025-08-14T23:25:03Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+
+
+class HostMonitoringV2Plugin(Plugin, CanReleaseResources):
+    _SUBSCRIBED_METHODS: Set[str] = {"*"}


Should just be connect, and network bound methods.
Efm should not be involved in the force_connect, accepts strategy and get_host_info_by_strategy pipeline for example

sophia-bq · 2025-08-14T23:32:00Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+        if connection is None:
+            raise AwsWrapperError(Messages.get_formatted("HostMonitoringV2Plugin.ConnectionNone", method_name))
+
+        is_enabled = WrapperProperties.FAILURE_DETECTION_ENABLED.get_bool(self._properties)


wondering if we could make the improvement to have this handled in initialization so if failure detection is not enabled in properties in initialization we remove methods of the execute pipeline from subscribed plugins - that way instead of each time something is executed we check the properties we only check them once?

sophia-bq · 2025-08-14T23:33:39Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+            raise AwsWrapperError(Messages.get_formatted("HostMonitoringV2Plugin.ConnectionNone", method_name))
+
+        is_enabled = WrapperProperties.FAILURE_DETECTION_ENABLED.get_bool(self._properties)
+        if not is_enabled or not self._plugin_service.is_network_bound_method(method_name):


see comments on subscribed plugins - we can filter on this earlier?

sophia-bq · 2025-08-14T23:34:53Z

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py

+                connection,
+                self._get_monitoring_host_info(),
+                self._properties,
+                WrapperProperties.FAILURE_DETECTION_TIME_MS.get_int(self._properties),


these should be constant after initialization - could also only gather the value once and store it

JuanLeee force-pushed the feat/efmv2 branch 2 times, most recently from 876d0d5 to 5a504a7 Compare August 7, 2025 16:33

JuanLeee changed the title ~~WIP: feat: efm v2 support~~ feat: efm v2 support Aug 7, 2025

JuanLeee force-pushed the feat/efmv2 branch 9 times, most recently from f6e9154 to 9727b6a Compare August 12, 2025 23:10

feat: efm v2 support

3358cd8

# Conflicts: # aws_advanced_python_wrapper/plugin_service.py # aws_advanced_python_wrapper/utils/atomic.py

JuanLeee force-pushed the feat/efmv2 branch from 9727b6a to 3358cd8 Compare August 13, 2025 00:05

karenc-bq reviewed Aug 13, 2025

View reviewed changes

docs/using-the-python-driver/using-plugins/UsingTheHostMonitoringPlugin.md Show resolved Hide resolved

karenc-bq reviewed Aug 13, 2025

View reviewed changes

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py Show resolved Hide resolved

karenc-bq reviewed Aug 14, 2025

View reviewed changes

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py Show resolved Hide resolved

karenc-bq reviewed Aug 14, 2025

View reviewed changes

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py Show resolved Hide resolved

karenc-bq reviewed Aug 14, 2025

View reviewed changes

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py Show resolved Hide resolved

karenc-bq reviewed Aug 14, 2025

View reviewed changes

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py Show resolved Hide resolved

karenc-bq reviewed Aug 14, 2025

View reviewed changes

aws_advanced_python_wrapper/host_monitoring_v2_plugin.py Show resolved Hide resolved

aaron-congo reviewed Aug 15, 2025

View reviewed changes

sophia-bq reviewed Aug 15, 2025

View reviewed changes

	start_monitoring_time_ns = ((current_time_ns + self._failure_detection_time_ns) // 10*9) 10**9
	start_monitoring_time_ns = current_time_ns + self._failure_detection_time_ns

	logger.debug("HostMonitorV2.HostNotResponding", self._host_info.host, self._failure_count)
	logger.debug("HostMonitorV2.HostNotResponding", self._host_info.host, self._failure_count)
	return

	HostMonitorV2.ExceptionAbortingConnection=[HostMonitorV2] Exception during aborting connection: '{}'
	HostMonitorV2.ExceptionAbortingConnection=[HostMonitorV2] Exception while aborting connection: '{}'

	MonitorServiceV2.ExceptionAbortingConnection=[MonitorServiceV2] Exception during aborting connection: '{}'
	MonitorServiceV2.ExceptionAbortingConnection=[MonitorServiceV2] Exception while aborting connection: '{}'

	> Since these two plugins are separate plugins, users may decide to use them together with a single connection. While this should not have any negative side effects, it is not recommended. It is recommended to use either the `host_monitoring` plugin, or the `host_monitoring_v2` plugin where it's needed.
	> Since these two plugins are separate plugins, users may decide to use them together with a single connection. While this should not have any negative side effects, it is not recommended. It is recommended to use either the `host_monitoring_v2` plugin, or the `host_monitoring` plugin where it's needed.



		class HostMonitoringV2Plugin(Plugin, CanReleaseResources):
		_SUBSCRIBED_METHODS: Set[str] = {"*"}

feat: efm v2 support #930

Are you sure you want to change the base?

feat: efm v2 support #930

Uh oh!

Conversation

JuanLeee commented Jul 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

karenc-bq Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaron-congo commented Aug 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JuanLeee commented Jul 31, 2025 •

edited

Loading

karenc-bq Aug 14, 2025 •

edited

Loading